fix: ignore deferred spans when delegating sampling decision #3209
Conversation
|
|
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #3209 +/- ##
=====================================
Coverage 80.8% 80.8%
=====================================
Files 128 128
Lines 23090 23118 +28
=====================================
+ Hits 18676 18700 +24
- Misses 4414 4418 +4 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
lbeschastny
force-pushed
the
fix/psrent-based-sampler-with-deferred-trace
branch
from
97b5b50 to
cb7930d
Compare
| #[cfg(feature = "jaeger_remote_sampler")] | ||
| use opentelemetry_http::HttpClient; | ||
|
|
||
| const TRACE_FLAG_DEFERRED: TraceFlags = TraceFlags::new(0x02); |
There was a problem hiding this comment.
https://w3c.github.io/trace-context/#trace-flags
I don't see such a flag definition in the official spec...
There was a problem hiding this comment.
Because it's not an official flag. It's a hack used across opentelemetry-rust project to mark spans which lack sampling information to distinguish between unsampled and not-yet-sampled ones.
Here are some examples:
opentelemetry_zipkin::propagatorsetsTRACE_FLAG_DEFERREDin several places:opentelemetry_jaeger_propagator::propagatorchecks it here:
opentelemetry-rust-contrib propagators are using the same approach as well, i.e.:
opentelemetry_aws::trace::xray_propagator: https://github.com/open-telemetry/opentelemetry-rust-contrib/blob/ef1a3669459a9e3bdd1c2cec3d6593662ae7d584/opentelemetry-aws/src/trace/xray_propagator.rs#L61opentelemetry_datadog: https://github.com/open-telemetry/opentelemetry-rust-contrib/blob/ef1a3669459a9e3bdd1c2cec3d6593662ae7d584/opentelemetry-datadog/src/lib.rs#L172
There was a problem hiding this comment.
It would've been better to provide a way to apply sampling inside a propagator when extracting tracing context.
But that's a separate thing that would require a major release.
3 participants
Propagators could leave spans in a deferred state. For example, the following valid set of b3-multi headers does eactly that:
but
ShouldSampleimplementation foropentelemetry_sdk::trace::sampler::Sampleronly checks that the parent span is active.The correct behaviour would be to fall back to delegate sampler when the parent spans has deferred flag set by a propagator.
Changes
Sampler::ParentBasedbranch in theshould_samplemethod now checks is the parent span hasTRACE_FLAG_DEFERREDset and filters all deferred spans.Merge requirement checklist
CHANGELOG.mdfiles updated for non-trivial, user-facing changes